-
Notifications
You must be signed in to change notification settings - Fork 10.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libc] Provide sys/queue.h #78081
[libc] Provide sys/queue.h #78081
Conversation
@llvm/pr-subscribers-libc Author: Petr Hosek (petrhosek) ChangesThis header first appeared in 4.4BSD and is provided by a number of C libraries including Newlib. Several of our embedded projects use this header and so to make LLVM libc a drop-in replacement, we need to provide it as well. For the initial commit, we only implement singly linked variants (SLIST and STAILQ). The doubly linked variants (LIST, TAILQ and CIRCLEQ) can be implemented in the future as needed. Full diff: https://github.com/llvm/llvm-project/pull/78081.diff 8 Files Affected:
diff --git a/libc/config/baremetal/arm/headers.txt b/libc/config/baremetal/arm/headers.txt
index e7be1fd80e8754..6ff51f9786772b 100644
--- a/libc/config/baremetal/arm/headers.txt
+++ b/libc/config/baremetal/arm/headers.txt
@@ -8,4 +8,5 @@ set(TARGET_PUBLIC_HEADERS
libc.include.stdlib
libc.include.string
libc.include.strings
+ libc.include.sys_queue
)
diff --git a/libc/config/baremetal/riscv/headers.txt b/libc/config/baremetal/riscv/headers.txt
index e7be1fd80e8754..6ff51f9786772b 100644
--- a/libc/config/baremetal/riscv/headers.txt
+++ b/libc/config/baremetal/riscv/headers.txt
@@ -8,4 +8,5 @@ set(TARGET_PUBLIC_HEADERS
libc.include.stdlib
libc.include.string
libc.include.strings
+ libc.include.sys_queue
)
diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 2c2d1b9b0fd155..895996453eb5ca 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -345,6 +345,12 @@ add_gen_header(
.llvm_libc_common_h
)
+add_header(
+ sys_queue
+ HDR
+ sys/queue.h
+)
+
add_gen_header(
sys_random
DEF_FILE sys/random.h.def
diff --git a/libc/include/sys/queue.h b/libc/include/sys/queue.h
new file mode 100644
index 00000000000000..50019c95d994a3
--- /dev/null
+++ b/libc/include/sys/queue.h
@@ -0,0 +1,161 @@
+//===-- Macros defined in sys/queue.h header file -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef __LLVM_LIBC_MACROS_SYS_QUEUE_MACROS_H
+#define __LLVM_LIBC_MACROS_SYS_QUEUE_MACROS_H
+
+#include <stddef.h>
+
+// Singly-linked list definitions.
+
+#define SLIST_HEAD(name, type) \
+ struct name { \
+ struct type *first; \
+ }
+
+#define SLIST_HEAD_INITIALIZER(head) \
+ { NULL }
+
+#define SLIST_ENTRY(type) \
+ struct { \
+ struct type *next; \
+ }
+
+// Singly-linked list access methods.
+
+#define SLIST_EMPTY(head) ((head)->first == NULL)
+#define SLIST_FIRST(head) ((head)->first)
+#define SLIST_NEXT(elem, field) ((elem)->field.next)
+
+#define SLIST_FOREACH(var, head, field) \
+ for ((var) = SLIST_FIRST(head); (var); (var) = SLIST_NEXT(var, field))
+
+#define SLIST_FOREACH_SAFE(var, head, field, tvar) \
+ for ((var) = SLIST_FIRST(head); \
+ (var) && ((tvar) = SLIST_NEXT(var, field), 1); (var) = (tvar))
+
+// Singly-linked list functions.
+
+#define SLIST_INIT(head) \
+ do { \
+ SLIST_FIRST(head) = NULL; \
+ } while (0)
+
+#define SLIST_INSERT_HEAD(head, elem, field) \
+ do { \
+ SLIST_NEXT(elem, field) = SLIST_FIRST(head); \
+ SLIST_FIRST(head) = (elem); \
+ } while (0)
+
+#define SLIST_INSERT_AFTER(slistelem, elem, field) \
+ do { \
+ SLIST_NEXT(elem, field) = SLIST_NEXT(slistelem, field); \
+ SLIST_NEXT(slistelem, field) = (elem); \
+ } while (0)
+
+#define SLIST_REMOVE_HEAD(head, field) \
+ do { \
+ SLIST_FIRST(head) = SLIST_NEXT(SLIST_FIRST(head), field); \
+ } while (0)
+
+#define SLIST_REMOVE_AFTER(elem, field) \
+ do { \
+ SLIST_NEXT(elem, field) = SLIST_NEXT(SLIST_NEXT(elem, field), field); \
+ } while (0)
+
+#define SLIST_REMOVE(head, elem, type, field) \
+ do { \
+ if (SLIST_FIRST(head) == (elem)) { \
+ SLIST_REMOVE_HEAD(head, field); \
+ } else { \
+ struct type *cur = SLIST_FIRST(head); \
+ while (SLIST_NEXT(elem, field) != (elem)) \
+ cur = SLIST_NEXT(elem, field); \
+ SLIST_REMOVE_AFTER(cur, field); \
+ } \
+ } while (0)
+
+// Singly-linked tail queue definitions.
+
+#define STAILQ_HEAD(name, type) \
+ struct name { \
+ struct type *first; \
+ struct type **last; \
+ }
+
+#define STAILQ_HEAD_INITIALIZER(head) \
+ { NULL, &(head).first }
+
+#define STAILQ_ENTRY(type) \
+ struct { \
+ struct type *next; \
+ }
+
+// Singly-linked tail queue access methods.
+
+#define STAILQ_EMPTY(head) ((head)->first == NULL)
+#define STAILQ_FIRST(head) ((head)->first)
+#define STAILQ_NEXT(elem, field) ((elem)->field.next)
+
+#define STAILQ_FOREACH(var, head, field) \
+ for ((var) = STAILQ_FIRST(head); (var); (var) = STAILQ_NEXT(var, field))
+
+#define STAILQ_FOREACH_SAFE(var, head, field, tvar) \
+ for ((var) = STAILQ_FIRST(head); \
+ (var) && ((tvar) = STAILQ_NEXT(var, field), 1); (var) = (tvar))
+
+// Singly-linked tail queue functions.
+
+#define STAILQ_INIT(head) \
+ do { \
+ STAILQ_FIRST(head) = NULL; \
+ (head)->last = &STAILQ_FIRST(head); \
+ } while (0)
+
+#define STAILQ_INSERT_HEAD(head, elem, field) \
+ do { \
+ if ((STAILQ_NEXT(elem, field) = STAILQ_FIRST(head)) == NULL) \
+ (head)->last = &STAILQ_NEXT(elem, field); \
+ STAILQ_FIRST(head) = (elem); \
+ } while (0)
+
+#define STAILQ_INSERT_TAIL(head, elem, field) \
+ do { \
+ STAILQ_NEXT(elem, field) = NULL; \
+ *(head)->last = (elem); \
+ (head)->last = &STAILQ_NEXT(elem, field); \
+ } while (0)
+
+#define STAILQ_INSERT_AFTER(head, listelem, elem, field) \
+ do { \
+ if ((STAILQ_NEXT(elem, field) = STAILQ_NEXT(listelem, field)) == NULL) \
+ (head)->last = &STAILQ_NEXT(elem, field); \
+ STAILQ_NEXT(listelem, next) = (elem); \
+ } while (0)
+
+#define STAILQ_REMOVE_HEAD(head, field) \
+ do { \
+ if ((STAILQ_FIRST(head) = STAILQ_NEXT(STAILQ_FIRST(head), field)) == NULL) \
+ (head)->last = &STAILQ_FIRST(head); \
+ } while (0)
+
+#define STAILQ_REMOVE(head, elem, type, field) \
+ do { \
+ if (STAILQ_FIRST(head) == (elem)) { \
+ STAILQ_REMOVE_HEAD(head, field); \
+ } else { \
+ struct type *cur = STAILQ_FIRST(head); \
+ while (STAILQ_NEXT(elem, field) != (elem)) \
+ cur = STAILQ_NEXT(cur, field); \
+ if ((STAILQ_NEXT(cur, field) = \
+ STAILQ_NEXT(STAILQ_NEXT(cur, field), field)) == NULL) \
+ (head)->last = &STAILQ_NEXT(cur, field); \
+ } \
+ } while (0)
+
+#endif // __LLVM_LIBC_MACROS_SYS_QUEUE_MACROS_H
diff --git a/libc/test/CMakeLists.txt b/libc/test/CMakeLists.txt
index 5e8231ef27f461..f22f2b183aca92 100644
--- a/libc/test/CMakeLists.txt
+++ b/libc/test/CMakeLists.txt
@@ -14,6 +14,7 @@ if(LIBC_TARGET_ARCHITECTURE_IS_GPU AND
return()
endif()
+add_subdirectory(include)
add_subdirectory(src)
add_subdirectory(utils)
diff --git a/libc/test/include/CMakeLists.txt b/libc/test/include/CMakeLists.txt
new file mode 100644
index 00000000000000..6083535e71eabc
--- /dev/null
+++ b/libc/test/include/CMakeLists.txt
@@ -0,0 +1,3 @@
+add_custom_target(libc_include_tests)
+
+add_subdirectory(sys)
diff --git a/libc/test/include/sys/CMakeLists.txt b/libc/test/include/sys/CMakeLists.txt
new file mode 100644
index 00000000000000..40477aa65ab493
--- /dev/null
+++ b/libc/test/include/sys/CMakeLists.txt
@@ -0,0 +1,11 @@
+add_libc_test(
+ queue_test
+ SUITE
+ libc_include_tests
+ SRCS
+ queue_test.cpp
+ DEPENDS
+ libc.include.sys_queue
+ libc.src.__support.char_vector
+ libc.src.__support.CPP.string
+)
diff --git a/libc/test/include/sys/queue_test.cpp b/libc/test/include/sys/queue_test.cpp
new file mode 100644
index 00000000000000..4d0daf6190f12d
--- /dev/null
+++ b/libc/test/include/sys/queue_test.cpp
@@ -0,0 +1,123 @@
+//===-- Unittests for queue -----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDSList-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/CPP/string.h"
+#include "src/__support/char_vector.h"
+#include "test/UnitTest/Test.h"
+
+#include <sys/queue.h>
+
+using LIBC_NAMESPACE::CharVector;
+using LIBC_NAMESPACE::cpp::string;
+
+namespace LIBC_NAMESPACE {
+
+TEST(LlvmLibcQueueTest, SList) {
+ struct Contrived {
+ char c;
+ SLIST_ENTRY(Contrived) entry;
+ };
+
+ SLIST_HEAD(Head, Contrived) head = SLIST_HEAD_INITIALIZER(head);
+
+ struct Contains : public testing::Matcher<Head> {
+ string s;
+ Contains(string s) : s(s) {}
+ bool match(Head head) {
+ Contrived *e;
+ CharVector v;
+ SLIST_FOREACH(e, &head, entry) { v.append(e->c); }
+ return s == v.c_str();
+ }
+ };
+
+ SLIST_INIT(&head);
+ ASSERT_TRUE(SLIST_EMPTY(&head));
+
+ Contrived e1 = {'a', {NULL}};
+ SLIST_INSERT_HEAD(&head, &e1, entry);
+
+ ASSERT_THAT(head, Contains("a"));
+
+ Contrived e2 = {'b', {NULL}};
+ SLIST_INSERT_AFTER(&e1, &e2, entry);
+
+ ASSERT_THAT(head, Contains("ab"));
+
+ Contrived *e, *tmp = NULL;
+ SLIST_FOREACH_SAFE(e, &head, entry, tmp) {
+ if (e == &e2) {
+ SLIST_REMOVE(&head, e, Contrived, entry);
+ }
+ }
+
+ ASSERT_THAT(head, Contains("a"));
+
+ while (!SLIST_EMPTY(&head)) {
+ e = SLIST_FIRST(&head);
+ SLIST_REMOVE_HEAD(&head, entry);
+ }
+
+ ASSERT_TRUE(SLIST_EMPTY(&head));
+}
+
+TEST(LlvmLibcQueueTest, STailQ) {
+ struct Contrived {
+ char c;
+ STAILQ_ENTRY(Contrived) entry;
+ };
+
+ STAILQ_HEAD(Head, Contrived) head = STAILQ_HEAD_INITIALIZER(head);
+
+ struct Contains : public testing::Matcher<Head> {
+ string s;
+ Contains(string s) : s(s) {}
+ bool match(Head head) {
+ Contrived *e;
+ CharVector v;
+ STAILQ_FOREACH(e, &head, entry) { v.append(e->c); }
+ return s == v.c_str();
+ }
+ };
+
+ STAILQ_INIT(&head);
+ ASSERT_TRUE(STAILQ_EMPTY(&head));
+
+ Contrived e1 = {'a', {NULL}};
+ STAILQ_INSERT_HEAD(&head, &e1, entry);
+
+ ASSERT_THAT(head, Contains("a"));
+
+ Contrived e2 = {'b', {NULL}};
+ STAILQ_INSERT_TAIL(&head, &e2, entry);
+
+ ASSERT_THAT(head, Contains("ab"));
+
+ Contrived e3 = {'c', {NULL}};
+ SLIST_INSERT_AFTER(&e2, &e3, entry);
+
+ ASSERT_THAT(head, Contains("abc"));
+
+ Contrived *e, *tmp = NULL;
+ STAILQ_FOREACH_SAFE(e, &head, entry, tmp) {
+ if (e == &e2) {
+ STAILQ_REMOVE(&head, e, Contrived, entry);
+ }
+ }
+
+ ASSERT_THAT(head, Contains("ac"));
+
+ while (!STAILQ_EMPTY(&head)) {
+ e = STAILQ_FIRST(&head);
+ STAILQ_REMOVE_HEAD(&head, entry);
+ }
+
+ ASSERT_TRUE(STAILQ_EMPTY(&head));
+}
+
+} // namespace LIBC_NAMESPACE
|
99ec187
to
fea0299
Compare
This header first appeared in 4.4BSD and is provided by a number of C libraries including Newlib. Several of our embedded projects use this header and so to make LLVM libc a drop-in replacement, we need to provide it as well. For the initial commit, we only implement singly linked variants (SLIST and STAILQ). The doubly linked variants (LIST, TAILQ and CIRCLEQ) can be implemented in the future as needed.
fea0299
to
7c725a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps worth noting in the commit message that man 7 queue
and man 3 slist
have more info.
Does FreeBSD have additional macros than other BSDs?
https://man.freebsd.org/cgi/man.cgi?query=SLIST_HEAD&sektion=3&manpath=FreeBSD%2010.2-RELEASE
libc/include/sys/queue.h
Outdated
#define SLIST_FOREACH(var, head, field) \ | ||
for ((var) = SLIST_FIRST(head); (var); (var) = SLIST_NEXT(var, field)) | ||
|
||
#define SLIST_FOREACH_SAFE(var, head, field, tvar) \ | ||
for ((var) = SLIST_FIRST(head); \ | ||
(var) && ((tvar) = SLIST_NEXT(var, field), 1); (var) = (tvar)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about other iteration methods such as SLIST_FOREACH_FROM
, SLIST_FOREACH_FROM_SAFE
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implemented all the remaining SLIST
macros.
I noticed that different C library implementations provide different sets of macros (I checked BSDs as well as glibc, newlib and bionic) so I started with a common subset. I'm happy to implement a superset if you think it'll be useful. |
At the very least, please add support for |
This matches the BSD and newlib implementations.
I initially planned to to use |
libc/include/sys/queue.h
Outdated
STAILQ_REMOVE_HEAD(head, field); \ | ||
} else { \ | ||
QUEUE_TYPEOF(type) *cur = STAILQ_FIRST(head); \ | ||
while (STAILQ_NEXT(elem, field) != (elem)) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be:
while (STAILQ_NEXT(cur, field) != (elem) \
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Ah, yeah in that case nevermind my earlier suggestion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks good with some organizational notes
libc/include/sys/queue.h
Outdated
#define QUEUE_TYPEOF(type) struct type | ||
#endif | ||
|
||
// Singly-linked list definitions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These macros seem overall fine, we normally organize our headers a bit differently. The macros should go in llvm-libc-macros/sys-queue-macros.h
, which will leave this header practically empty. That's fine though, since that will allow you to use the path include/llvm-libc-macros/sys-queue-macros.h
in your test which should make it guaranteed to grab the right file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -8,4 +8,5 @@ set(TARGET_PUBLIC_HEADERS | |||
libc.include.stdlib | |||
libc.include.string | |||
libc.include.strings | |||
libc.include.sys_queue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be nice to add this header to the other targets as well since it should still work, and that will make sure that it's being tested by our upstream builders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Looks like there's a presubmit build failure:
|
SLIST_REMOVE_HEAD(head, field); \ | ||
} else { \ | ||
QUEUE_TYPEOF(type) *cur = SLIST_FIRST(head); \ | ||
while (SLIST_NEXT(elem, field) != (elem)) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should also be
while (SLIST_NEXT(cur, field) != (elem)) \
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps worth once more unto the breach to appease the code format linter, but LGTM
libc.src.__support.char_vector | ||
libc.src.__support.CPP.string | ||
COMPILE_OPTIONS | ||
-Wno-gnu-statement-expression-from-macro-expansion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps worth a comment //why// this is necessary. (I have no problem adding, and //I// know why, but future travelers might not know immediately, and I'm likely to forget).
const __typeof(((type *)0)->member) *__ptr = ptr; \ | ||
(type *)(void *)((const char *)__ptr - offsetof(type, member)); \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: the Linux kernel has some updates to this macro, such as:
I don't think we need those per-se at the moment, but I do wonder when we will.
I believe this change made the libc on AMDGPU buildbot fail: https://lab.llvm.org/staging/#/builders/11/builds/608. |
The error message indicates that a file that was not compiled with |
cebe4de should fix it. |
This header first appeared in 4.4BSD and is provided by a number of C libraries including Newlib. Several of our embedded projects use this header and so to make LLVM libc a drop-in replacement, we need to provide it as well.
For the initial commit, we only implement singly linked variants (SLIST and STAILQ). The doubly linked variants (LIST, TAILQ and CIRCLEQ) can be implemented in the future as needed.